From 863d96075d70b11d9020393c7f29124e33a0d48a Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Mon, 15 Feb 2021 10:00:03 +0000 Subject: [PATCH] d/patches: Never mark rendering differences as accepted, only tolerated Upstream consider absolutely any difference in rendering to be an error, so we should always record the diff for inspection. --- debian/close-enough.keyfile | 2 - ...ces-Report-how-much-the-images-diffe.patch | 7 +-- ...ow-minor-differences-to-be-tolerated.patch | 59 +++++-------------- 3 files changed, 18 insertions(+), 50 deletions(-) diff --git a/debian/close-enough.keyfile b/debian/close-enough.keyfile index f3a214beae..1ae6ff92d9 100644 --- a/debian/close-enough.keyfile +++ b/debian/close-enough.keyfile @@ -1,5 +1,3 @@ [reftest] -accepted-diff-level=1 -accepted-diff-pixels=50 tolerated-diff-level=10 tolerated-diff-pixels=100 diff --git a/debian/patches/reftest_compare_surfaces-Report-how-much-the-images-diffe.patch b/debian/patches/reftest_compare_surfaces-Report-how-much-the-images-diffe.patch index 38fecd48d0..f9c47e1f68 100644 --- a/debian/patches/reftest_compare_surfaces-Report-how-much-the-images-diffe.patch +++ b/debian/patches/reftest_compare_surfaces-Report-how-much-the-images-diffe.patch @@ -2,10 +2,9 @@ From: Simon McVittie Date: Sat, 13 Feb 2021 18:26:24 +0000 Subject: reftest_compare_surfaces: Report how much the images differ -Some of the reftests don't produce *identical* results on all -architectures, but do produce results that are visually -indistinguishable. Report how many pixels differ and by how much, so we -can get an idea of what's a rounding error and what's a serious problem. +In unattended/non-interactive/autobuilder environments where the images +are not trivially accessible, this provides a way to distinguish between +totally different rendering and more subtle issues. Signed-off-by: Simon McVittie Forwarded: https://gitlab.gnome.org/GNOME/gtk/-/merge_requests/3195 diff --git a/debian/patches/reftests-Allow-minor-differences-to-be-tolerated.patch b/debian/patches/reftests-Allow-minor-differences-to-be-tolerated.patch index 1fd7144cab..070a1c7bcd 100644 --- a/debian/patches/reftests-Allow-minor-differences-to-be-tolerated.patch +++ b/debian/patches/reftests-Allow-minor-differences-to-be-tolerated.patch @@ -9,18 +9,10 @@ Each .ui or .node reftest can have an accompanying .keyfile file like this: [reftest] - accepted-diff-level=2 - accepted-diff-pixels=100 tolerated-diff-level=20 tolerated-diff-pixels=1000 -If the number of pixels that differ from the reference is no more than -accepted-diff-pixels, and each channel in each of those pixels differs -from the reference by no more than accepted-diff-level, then we consider -that to be a full success, and don't even save the .diff.png for -analysis. - -If that check fails, but the number of pixels that differ is no more +If the image differs, but the number of pixels that differ is no more than tolerated-diff-pixels and the differences are no more than tolerated-diff-level, then we treat it as a success with warnings, save the .diff.png for analysis, and use g_test_incomplete() to record the @@ -30,13 +22,13 @@ Signed-off-by: Simon McVittie Forwarded: https://gitlab.gnome.org/GNOME/gtk/-/merge_requests/3195 Applied-upstream: no, upstream want reftests to be a strict pass/fail with identical results required --- - testsuite/gsk/compare-render.c | 43 ++++++++++++++++++++++++++++++++++++-- - testsuite/reftests/gtk-reftest.c | 43 ++++++++++++++++++++++++++++++++++++-- + testsuite/gsk/compare-render.c | 31 ++++++++++++++++++++++++++++++- + testsuite/reftests/gtk-reftest.c | 32 +++++++++++++++++++++++++++++++- testsuite/reftests/image-compare.c | 2 +- - 3 files changed, 83 insertions(+), 5 deletions(-) + 3 files changed, 62 insertions(+), 3 deletions(-) diff --git a/testsuite/gsk/compare-render.c b/testsuite/gsk/compare-render.c -index da6f9e2..a4afd75 100644 +index da6f9e2..237b8c4 100644 --- a/testsuite/gsk/compare-render.c +++ b/testsuite/gsk/compare-render.c @@ -98,6 +98,12 @@ get_output_file (const char *file, @@ -52,7 +44,7 @@ index da6f9e2..a4afd75 100644 static void save_image (cairo_surface_t *surface, const char *test_name, -@@ -242,12 +248,45 @@ main (int argc, char **argv) +@@ -242,12 +248,35 @@ main (int argc, char **argv) if (diff_surface) { @@ -60,18 +52,12 @@ index da6f9e2..a4afd75 100644 + GKeyFile *keyfile = g_key_file_new (); + guint64 tolerated_diff = 0; + guint64 tolerated_pixels = 0; -+ guint64 accepted_diff = 0; -+ guint64 accepted_pixels = 0; + + if (keyfile_path != NULL && g_file_test (keyfile_path, G_FILE_TEST_EXISTS)) + { + GError *error = NULL; + g_key_file_load_from_file (keyfile, keyfile_path, G_KEY_FILE_NONE, &error); + g_assert_no_error (error); -+ accepted_diff = g_key_file_get_uint64 (keyfile, "reftest", "accepted-diff-level", NULL); -+ g_print ("Maximum difference accepted: %" G_GUINT64_FORMAT " levels\n", accepted_diff); -+ accepted_pixels = g_key_file_get_uint64 (keyfile, "reftest", "accepted-diff-pixels", NULL); -+ g_print ("Different pixels accepted: %" G_GUINT64_FORMAT "\n", accepted_pixels); + tolerated_diff = g_key_file_get_uint64 (keyfile, "reftest", "tolerated-diff-level", NULL); + g_print ("Maximum difference tolerated: %" G_GUINT64_FORMAT " levels\n", tolerated_diff); + tolerated_pixels = g_key_file_get_uint64 (keyfile, "reftest", "tolerated-diff-pixels", NULL); @@ -81,17 +67,12 @@ index da6f9e2..a4afd75 100644 g_print ("%u (out of %u) pixels differ from reference by up to %u levels\n", pixels_changed, pixels, max_diff); -- save_image (diff_surface, node_file, ".diff.png"); -+ if (max_diff > accepted_diff || pixels_changed > accepted_pixels) -+ save_image (diff_surface, node_file, ".diff.png"); -+ + save_image (diff_surface, node_file, ".diff.png"); cairo_surface_destroy (diff_surface); - success = FALSE; + -+ if (max_diff <= accepted_diff && pixels_changed <= accepted_pixels) -+ g_print ("differences are within acceptable range\n"); -+ else if (max_diff <= tolerated_diff && pixels_changed <= tolerated_pixels) -+ g_print ("not right, but close enough\n"); ++ if (max_diff <= tolerated_diff && pixels_changed <= tolerated_pixels) ++ g_print ("not right, but close enough?\n"); + else + g_test_fail (); + @@ -101,7 +82,7 @@ index da6f9e2..a4afd75 100644 } diff --git a/testsuite/reftests/gtk-reftest.c b/testsuite/reftests/gtk-reftest.c -index 6ef17aa..791e95c 100644 +index 6ef17aa..3ef6d32 100644 --- a/testsuite/reftests/gtk-reftest.c +++ b/testsuite/reftests/gtk-reftest.c @@ -290,6 +290,12 @@ save_image (cairo_surface_t *surface, @@ -117,7 +98,7 @@ index 6ef17aa..791e95c 100644 static void test_ui_file (GFile *file) { -@@ -326,10 +332,43 @@ test_ui_file (GFile *file) +@@ -326,10 +332,34 @@ test_ui_file (GFile *file) if (diff_image) { @@ -125,18 +106,12 @@ index 6ef17aa..791e95c 100644 + GKeyFile *keyfile = g_key_file_new (); + guint64 tolerated_diff = 0; + guint64 tolerated_pixels = 0; -+ guint64 accepted_diff = 0; -+ guint64 accepted_pixels = 0; + + if (keyfile_path != NULL) + { + GError *error = NULL; + g_key_file_load_from_file (keyfile, keyfile_path, G_KEY_FILE_NONE, &error); + g_assert_no_error (error); -+ accepted_diff = g_key_file_get_uint64 (keyfile, "reftest", "accepted-diff-level", NULL); -+ g_test_message ("Maximum difference accepted: %" G_GUINT64_FORMAT " levels", accepted_diff); -+ accepted_pixels = g_key_file_get_uint64 (keyfile, "reftest", "accepted-diff-pixels", NULL); -+ g_test_message ("Different pixels accepted: %" G_GUINT64_FORMAT, accepted_pixels); + tolerated_diff = g_key_file_get_uint64 (keyfile, "reftest", "tolerated-diff-level", NULL); + g_test_message ("Maximum difference tolerated: %" G_GUINT64_FORMAT " levels", tolerated_diff); + tolerated_pixels = g_key_file_get_uint64 (keyfile, "reftest", "tolerated-diff-pixels", NULL); @@ -145,16 +120,12 @@ index 6ef17aa..791e95c 100644 + g_test_message ("%u (out of %u) pixels differ from reference by up to %u levels", pixels_changed, pixels, max_diff); -- save_image (diff_image, ui_file, ".diff.png"); -- g_test_fail (); + -+ if (max_diff > accepted_diff || pixels_changed > accepted_pixels) -+ save_image (diff_image, ui_file, ".diff.png"); + save_image (diff_image, ui_file, ".diff.png"); +- g_test_fail (); + -+ if (max_diff <= accepted_diff && pixels_changed <= accepted_pixels) -+ g_test_message ("differences are within acceptable range"); -+ else if (max_diff <= tolerated_diff && pixels_changed <= tolerated_pixels) -+ g_test_incomplete ("not right, but close enough"); ++ if (max_diff <= tolerated_diff && pixels_changed <= tolerated_pixels) ++ g_test_incomplete ("not right, but close enough?"); + else + g_test_fail (); + -- 2.30.2